-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Refactor string interpolators and error message handling #16384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
b742e83
to
73ed7f2
Compare
- Revise string interpolators; `em` and `exm` nor produce messages, whereas `i`, `e`, and `ex` produce strings. - Use Message intead of () => String in some places.
Let them take Messages instead of Strings. We now are in a situation where the `e` interpolator is only used within `messages` itself. This means we can hopefully roll its behavior into the `Message` logic.
f959eef
to
ff705aa
Compare
This will be the basis on which we modify contexts for printing diagnsotics.
4ca606a
to
e64014f
Compare
In particular: The missingArg message of ImplicitSearchError becomes a regular Message now.
e64014f
to
052b1cd
Compare
052b1cd
to
02eb18f
Compare
9a75d3f
to
cdabf41
Compare
That leaves just two interpolators: - i for strings - em for error messages (and other diagnostics)
Also: use exclusively Message insteax of String for error handling in Scanner and Parser.
Using Message instead of String for arguments of report methods. String versions are still retained for external interop. But they should be used only if the argument is a simple string literal.
- Use aggregation instead of inheritance - Encapsulate explanations inside Seen
Who would like to give this a review? To be sure, given the change set a lof of it will have to be cursory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been reading the diffs as you've been pushing them. The only thing I didn't reread when code moved files, so I missed any changes in there (if any). But the rest LGTM.
OK, great! The code that moved from Formatting to Message was unchanged except for straightforward refactorings. |
The idea is that we want to concentrate error handling logic in the
Message
type instead of dispersing it over variousstring interpolators.
report
methods.Message
.From the doc comment for
Message
:Tips for error message generation
em
interpolator for error messages. It's defined in core.Decorators.error
orwarning
(not for the other variants),but the string should not be interpolated or composed of objects that require a
Context for evaluation.
use
i
and make sure they are defined as def's instead of vals. That way, thepossibly expensive interpolation will performed only in the case where the message
is eventually printed. Note: At least during typer, it's common for messages
to be discarded without being printed. Also, by making them defs, you ensure that
they will be evaluated in the Message context, which makes formatting safer
and more robust.
a new
Message
in filemessages.scala
and use that instead. The advantage is that thesemessages have unique IDs that can be referenced elsewhere.